Skip to content

docs links test improvements - #20797

Merged
pondrejk merged 1 commit into
SatelliteQE:masterfrom
rmynar:sat-41822
Mar 3, 2026
Merged

docs links test improvements#20797
pondrejk merged 1 commit into
SatelliteQE:masterfrom
rmynar:sat-41822

Conversation

@rmynar

@rmynar rmynar commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Improvements to test_positive_documentation_links:

  1. The test is parametrized, so there's one test per page, which makes it easier to analyze results
  2. There's a module scoped session which keeps user logged in during testing links on each page (saves time)
  3. Explicit redirect to single page in staging is no more needed since also production links lead to single page version (Link to the single-page version of docs RedHatSatellite/foreman_theme_satellite#202)

Summary by Sourcery

Parametrize documentation link E2E test by page and reuse a shared UI session to speed up execution while keeping per-page assertions and logging.

Tests:

  • Parametrize test_positive_documentation_links over individual pages instead of looping internally, producing separate test cases per page.
  • Introduce a module-scoped UI session fixture to reuse a logged-in session across documentation link checks and reduce test runtime.

Summary by Sourcery

Parametrize the documentation links E2E test per page and reuse a shared UI session to speed up execution and improve failure isolation.

Tests:

  • Split test_positive_documentation_links into parametrized per-page test cases for clearer reporting of broken links.
  • Introduce a module-scoped UI session fixture to reuse an authenticated session across documentation link checks.
  • Simplify link collection and broken-link reporting to operate on a single page at a time with clearer error messages.

@sourcery-ai

sourcery-ai Bot commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Parametrizes the documentation links E2E test per page and introduces a shared module-scoped UI session to speed up execution while preserving per-page link collection and validation logic.

File-Level Changes

Change Details Files
Parametrize documentation links test by page instead of looping internally.
  • Extract the list of pages to a module-level constant used for parametrization.
  • Apply pytest.mark.parametrize over the page argument to create one test instance per page.
  • Refactor the test body to operate on a single page and log links per-page instead of aggregating across all pages.
tests/foreman/ui/test_documentation_links.py
Reuse a module-scoped UI session fixture across paginated documentation link checks.
  • Introduce a module-scoped session fixture that yields a shared ui_session from module_target_sat.
  • Update test_positive_documentation_links to consume module_target_sat and the shared session fixture.
  • Move per-page navigation and documentation_links() collection to use the shared session instead of opening a new ui_session per test run.
tests/foreman/ui/test_documentation_links.py
Simplify per-page link validation and staging/production URL handling.
  • Remove defaultdict-based aggregation of all_links and pages_with_broken_links in favor of simple lists per page.
  • Adjust handling of Non-GA Satellite versions to drop explicit html-single redirection, relying on upstream behavior.
  • Change final assertion and error message to report broken links scoped to the single page under test.
tests/foreman/ui/test_documentation_links.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@rmynar rmynar added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.19.z labels Feb 12, 2026
@rmynar rmynar self-assigned this Feb 12, 2026
else:
view = page_object.navigate_to(page_object, 'All')
# Get the doc links present on the page.
all_links[page] = view.documentation_links()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the test is parametrized, all_links and pages_with_broken_links can be one-dimensional arrays instead of dicts.

Suggested change
all_links[page] = view.documentation_links()
all_links = view.documentation_links()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I replaced pages_with_broken_links dict with broken_links list

@adamlazik1

Copy link
Copy Markdown
Contributor

trigger: test-robottelo
pytest: tests/foreman/ui/test_documentation_links.py

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 14458
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_documentation_links.py --external-logging
Test Result : ============ 25 failed, 1 passed, 84 warnings in 1087.94s (0:18:07) ============

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Feb 25, 2026
@rmynar
rmynar marked this pull request as ready for review February 25, 2026 13:28

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider renaming the module-scoped fixture from session to something more specific like ui_session to avoid confusion with other common session concepts (e.g., requests.Session, pytest session scope) and improve readability.
  • The logic for fetching and normalizing documentation links makes two requests.get calls in the non-GA case (first for redirect resolution and then for status); consider reusing the first response or extracting a small helper to avoid duplicate network calls and clarify the flow.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider renaming the module-scoped fixture from `session` to something more specific like `ui_session` to avoid confusion with other common `session` concepts (e.g., `requests.Session`, pytest session scope) and improve readability.
- The logic for fetching and normalizing documentation links makes two `requests.get` calls in the non-GA case (first for redirect resolution and then for status); consider reusing the first response or extracting a small helper to avoid duplicate network calls and clarify the flow.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@adamlazik1

Copy link
Copy Markdown
Contributor

It appears the version for which the stage docs do not exist (6.20) is also tested, or at least in PRT. Should a solution be devised to avoid this scenario?

@rmynar rmynar added No-CherryPick PR doesnt need CherryPick to previous branches and removed CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing PRT-Failed Indicates that latest PRT run is failed for the PR 6.19.z labels Feb 25, 2026
@rmynar

rmynar commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

I did a workaround - I created a cherry-pick of this PR into 6.19.z branch. The PRT passed there, because 6.19 docs are already present in staging.

@adamlazik1

Copy link
Copy Markdown
Contributor

It still is probably a good idea to have this on master too so it gets branched automatically in future versions. Should we merge this as is or can we prevent this test to trigger on the stream snaps?

@adamlazik1 adamlazik1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after offline discussion.

# Get the end url first and then update it in later part of test.
if module_target_sat.hostname in link:
link = requests.get(link, verify=False).url
link = link.replace('https://docs.redhat.com', settings.robottelo.stage_docs_url)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel instead of hardcoding this prod docs url here, we can move it to settings as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same concern, but there's already some work going on within SAT-41490. The documentation portal can be customizable. I expect to modify this hardcoded part when creating new test(s) for this new feature.

Comment thread tests/foreman/ui/test_documentation_links.py
@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Feb 26, 2026
@lpramuk

lpramuk commented Feb 27, 2026

Copy link
Copy Markdown
Contributor
trigger: test-robottelo
pytest: tests/foreman/ui/test_documentation_links.py
env:
  ROBOTTELO_server__version__release: 6.19.0
  ROBOTTELO_server__version__snap: '2.0'

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 14535
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_documentation_links.py --external-logging
Test Result : ============ 25 failed, 1 passed, 94 warnings in 1814.60s (0:30:14) ============

@SatelliteQE SatelliteQE deleted a comment from Satellite-QE Feb 27, 2026
@SatelliteQE SatelliteQE deleted a comment from Satellite-QE Feb 27, 2026
@lpramuk

lpramuk commented Feb 27, 2026

Copy link
Copy Markdown
Contributor
trigger: test-robottelo
pytest: tests/foreman/ui/test_documentation_links.py
env:
  ROBOTTELO_server__version__release: 6.19.0
  ROBOTTELO_server__version__snap: '2.0'
  ROBOTTELO_server__deploy_arguments__deploy_sat_version: 6.19.0
  ROBOTTELO_server__deploy_arguments__deploy_snap_version: '2.0'

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 14542
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_documentation_links.py --external-logging
Test Result : ================ 26 passed, 106 warnings in 1601.69s (0:26:41) =================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Feb 27, 2026
@pondrejk
pondrejk merged commit 92db7fe into SatelliteQE:master Mar 3, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No-CherryPick PR doesnt need CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants